Skip to content

[IMP] Bound the CPU, memory and time of an indexing pass - #1

Open
moylop260 wants to merge 5 commits into
mainfrom
main-index-resource-limits-moy
Open

[IMP] Bound the CPU, memory and time of an indexing pass#1
moylop260 wants to merge 5 commits into
mainfrom
main-index-resource-limits-moy

Conversation

@moylop260

Copy link
Copy Markdown
Contributor

Why

Left alone the indexer sizes itself against the whole machine. Measured inside a Vauxoo
container on an 18 GB MacBook (Docker Desktop VM of 11.9 GiB):

codebase-memory-mcp cli --index-worker    428% CPU    9.0 GB RSS  (73.9% of the VM)
container irc190_01:  NanoCpus=0  CpuShares=0  Memory=0

That is every core the container can see and three quarters of the VM, which leaves the laptop
swapping (kernel_task at 287%, 128 MB free). Containers are normally started with no cgroup
limit at all, so nothing below this tool stops it either: docker update --cpus=5 from the
outside is a patch, the contention has to come from here.

What

Every indexing pass now gets a quarter of the machine. Each value is resolved from the command
line first, then from an already exported CBM_* variable, then from the machine, and the
resolved value and its source are logged:

workers=1 source=default usable_cpus=5
memory_mb=2816 source=default total_ram_mb=11264 enforce=yes
index_worker_timeout_s=7200 source=default
Flag Handed to the child as Default
--workers N CBM_WORKERS a quarter of the usable CPUs, never below 1
--max-memory-mb N CBM_MEM_BUDGET_MB and the enforced cap a quarter of the usable memory, never below 512 MB
--index-timeout N CBM_INDEX_WORKER_TIMEOUT_S 7200 (two hours)
--no-enforce-memory off: the cap is enforced

"Usable" means what the container may use, not what the host has: the cgroup CPU quota and
the cgroup memory limit, falling back to the CPU affinity mask and /proc/meminfo. On the
measured container docker update --cpus=5 reads back as 5, where os.cpu_count() keeps
reporting 10.

The memory cap is enforced here, not by the environment variable

CBM_MEM_BUDGET_MB is not a cap, and setting it would not have delivered what was asked.
It only tells codebase-memory-mcp when to log mem.pressure and purge its allocator — the run
above reported mem.init budget_mb=2986 total_ram_mb=11946 source=ram_fraction and still
reached 9.0 GB RSS. Reading codebase-memory-mcp 0.9.1-rc.1 confirms it: src/foundation/mem.c
tracks RSS and warns, it never enforces, and none of the 45 CBM_* variables bounds it.

The portable OS mechanisms do not fit either:

  • RLIMIT_AS / RLIMIT_DATA count the address space mimalloc reserves without ever touching
    it, so the pass would die well below its real usage.
  • RLIMIT_RSS is a no-op on Linux.
  • Writing to the cgroup needs privileges a container running as odoo does not have.

So the cap is enforced from the parent: the RSS of the child process tree is sampled while
the pass runs — the parsing happens in the --index-worker child, so watching only the process
this tool started would miss it — and the tree is killed (SIGTERM, then SIGKILL after a grace)
when it crosses the limit:

memory_cap_exceeded rss_mb=3012 limit_mb=2816 processes=2; killing the pass
Batch 3: the indexing pass reached 3012 MB, over the 2816 MB cap. Lower --batch-size so each
pass parses fewer modules, or raise --max-memory-mb when the machine can afford it

Recovery is the mechanism this tool already ships and documents: the batches are cumulative, so
everything indexed so far stays in the graph and a smaller --batch-size picks up from there.
The tree is signalled member by member instead of through its process group on purpose — putting
the pass in a session of its own would stop Ctrl-C from reaching a run that takes hours.

Verified

  • 83 tests pass (pytest), pre-commit run --all-files clean.
  • Readers checked against the real container irc190_01, not only mocked: cgroup_cpu_quota
    returns 5.0 for the --cpus=5 applied to it, cgroup_memory_limit 11811160064,
    default_workers 1, default_memory_mb 2816.
  • The watchdog checked there too: a parent + a growing grandchild, cap 200 MB →
    memory_cap_exceeded rss_mb=206 limit_mb=200 processes=2, the grandchild released its memory,
    and the single-process case returns in 1.13 s.

Worth a second opinion

  • Defaults are conservative on a limited container. With --cpus=5 in place, a quarter is
    workers=1. That is what "a quarter" means and it is one flag away, but it is slower than the
    status quo — say so if the fraction should have a floor above 1.
  • A cap that fires is a failed pass. Today the run takes the machine down instead; this turns
    that into a bounded, explainable failure with a documented way forward. It is still a
    behaviour change for anyone whose instance legitimately needs more than a quarter of RAM.
  • Sampling every 5 s can let a very fast allocation overshoot the cap before the next sample.

…ndexing pass

Left alone the indexer sizes itself against the whole machine. Measured inside a
Vauxoo container on an 18 GB MacBook (Docker Desktop VM of 11.9 GiB):

    codebase-memory-mcp cli --index-worker    428% CPU    9.0 GB RSS (73.9%)
    container irc190_01:  NanoCpus=0  CpuShares=0  Memory=0

That is every core the container can see and three quarters of the VM, which
leaves the laptop swapping. Containers are normally started with no cgroup limit
at all, so the contention has to come from here. Every pass now gets a quarter of
the machine, each value overridable on the command line and then by an already
exported CBM_* variable:

    --workers N          CBM_WORKERS                 a quarter of the usable CPUs
    --max-memory-mb N    CBM_MEM_BUDGET_MB           a quarter of the usable memory
    --index-timeout N    CBM_INDEX_WORKER_TIMEOUT_S  7200 (two hours)

"Usable" is what the container may use, not what the host has: the cgroup CPU
quota ("docker update --cpus=5" reads back as 5, where os.cpu_count() keeps
reporting 10) and the cgroup memory limit, falling back to the affinity mask and
/proc/meminfo.

CBM_MEM_BUDGET_MB is not a cap and cannot be used as one. It only tells
codebase-memory-mcp when to log mem.pressure and purge its allocator: the run
above reported "mem.init budget_mb=2986 total_ram_mb=11946 source=ram_fraction"
and still reached 9.0 GB RSS. codebase-memory-mcp exposes no variable that bounds
RSS, and the portable OS mechanisms do not fit either -- RLIMIT_AS/RLIMIT_DATA
count the address space mimalloc reserves without touching it, RLIMIT_RSS is a
no-op on Linux, and writing to the cgroup needs privileges a container running as
odoo does not have.

So the cap is enforced from the parent: the RSS of the child process tree is
sampled while the pass runs -- the parsing happens in the --index-worker child, so
watching only the process we started would miss it -- and the tree is killed when
it crosses the limit. Recovery is the mechanism this tool already ships: the
batches are cumulative, so a smaller --batch-size picks up where the killed pass
left off. --no-enforce-memory goes back to measuring only.
@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

…e exists

The CI matrix runs the suite on Windows too, where signal.SIGKILL does not exist,
/proc is absent and "ps" is not a command: the watchdog would have raised
AttributeError on the kill path and, worse, sampled an empty process table and
reported a cap it was never going to enforce. It now says so once
(memory_cap=unenforceable) and runs the pass unbounded, and the tests that need a
real process table are skipped there instead of hanging on a process nothing kills.
Reading codebase-memory-mcp shows two of the three knobs were already its job:

* CBM_MEM_BUDGET_MB: mem.c already scales the budget with the machine (25% at
  or below 16 GB, 35% at or below 32 GB, 50% above), so on the measured 11.9 GiB
  VM it resolves to the same 2986 MB a "quarter of RAM" rule produces. Exporting
  it changed nothing.
* CBM_INDEX_WORKER_TIMEOUT_S: it is a NO-PROGRESS window, not a time budget.
  index_supervisor.c kills a worker that logs nothing for 15 minutes and every
  progress line resets the clock, so setting 7200 did not give a pass two hours,
  it made the hang detector four times slower to fire. The flag stays, unset by
  default and documented for what it is.

Only CBM_WORKERS was genuinely missing: cbm_default_worker_count(initial=true)
returns total_cores on purpose ("Use all cores for initial indexing -- user is
waiting"), counted with sysconf(_SC_NPROCESSORS_ONLN), which inside a container
reports host CPUs. codebase-memory-mcp documents that gap where it reads the
override and delegates the cgroup quota to its caller. That is what is left here.

The parent-side RSS watchdog is gone, and a real run shows why keeping it would
have been worse than useless: indexing a 325-module instance it reported
"peak_rss_mb=15" while the worker died with signal 9. The pass does not run as a
descendant of the process we spawn ("Preparing one-shot local CBM command..."),
so the sampler was watching the wrong tree and would never have fired. A memory
ceiling belongs to the kernel anyway -- "docker run --memory=3g --cpus=2" costs
one flag, covers everything in the container, and --cpus is picked up by the
worker count above.

Net effect: 400 lines of userspace supervision replaced by the one value
codebase-memory-mcp cannot work out for itself, and no new dependency.
Indexing only .py cut an Odoo instance from 15860 files to 5681 and still ran a
5 GiB container out of memory, so the earlier claim that it takes the peak from
~14 GB to ~5 GB was wrong. The peak tracks extracted nodes (123023 nodes at
4880 MB, about 40 KB each), and nodes come almost entirely from Python, so the
extensions are a weak lever and the docs now say so.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants